Skip to content

(feat)O3-2971: Adding conformation modal for an empty form. #464

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

UNCANNY69
Copy link

@UNCANNY69 UNCANNY69 commented Feb 9, 2025

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

This PR adds a feature which triggers a conformation modal when User submits an empty form.

Screenshots

OpenMRS.-.Google.Chrome.2025-02-11.23-09-36.mp4

Related Issue

Other

@UNCANNY69 UNCANNY69 changed the title Adding conformation modal for empty form (feat)O3-2971: Adding conformation modal for an empty form. Feb 9, 2025
@UNCANNY69
Copy link
Author

I made many changes to the form-factory-provider , I would like it to be reviewed in case it can be designed better @NethmiRodrigo @samuelmale @denniskigen

@UNCANNY69
Copy link
Author

Reated PR:

openmrs/openmrs-esm-patient-chart#2237

@UNCANNY69
Copy link
Author

I made the changes according to the discussion in the JIRA, can it be reviewed again @denniskigen @samuelmale @NethmiRodrigo

@UNCANNY69 UNCANNY69 requested a review from samuelmale February 12, 2025 16:52
@UNCANNY69 UNCANNY69 requested a review from samuelmale April 21, 2025 11:29
@UNCANNY69 UNCANNY69 requested a review from vasharma05 April 22, 2025 09:55
// Validate all forms
const isValid = forms.every((formContext) => validateForm(formContext));
// Check if the form is empty
const isEmpty = forms.every((formContext) => validateEmptyForm(formContext));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to evaluate isEmpty if the form is invalid.

if (isEmpty && isValid) {
if (handleEmptyFormSubmission && typeof handleEmptyFormSubmission === 'function') {
try {
await handleEmptyFormSubmission();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should basically launch a modal prompting the user. Based on the user's decision, we should either save an empty encounter by submitting the form or close the form. Here is an example.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in the current scenario the user is not allowed to submit empty forms, thus there is no saving the form.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair! In either case, we should expect handleEmptyFormSubmission to promise some kind of boolean value; true we discard the form otherwise we continue editing. If the promise is rejected -- continue editing the form.

@@ -46,6 +46,11 @@ export function validateForm(context: FormContextProps) {
return errors.length === 0;
}

export function validateEmptyForm(context: FormContextProps) {
const { methods: { formState: { dirtyFields } } } = context;
return Object.keys(dirtyFields).length === 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you filter out transient fields?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants